Ensure citation html builder accepts dois with doi: prefix#218
Ensure citation html builder accepts dois with doi: prefix#218
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes dataset citation rendering for DOIs that may be stored with a doi: prefix (as produced by the API), ensuring the citation builder generates a correct DOI link and avoids Shiny render errors in the citation subpanel.
Changes:
- Rename
build_dataset_citation_text()tobuild_dataset_citation_html()and update callers/tests accordingly. - Expand DOI detection to accept optional
doi:prefix and normalize the DOI for display/linking. - Adjust citation plain-text extraction to parse from wrapped HTML content before copying.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
R/detail_dataset.R |
Updates DOI detection/normalization and switches detail view citation rendering to the new HTML builder. |
tests/testthat/test_detail_dataset.R |
Renames tests to the new function and adds coverage for doi:-prefixed DOIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nks for datasets instead of vb_code cite links
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mbjones
left a comment
mbjones
left a comment
There was a problem hiding this comment.
LGTM. I reviewed and ran all tests, all clean. I merged in the documentation changes from the other related branch, and bumped the version to 1.0.2. Because there are not even any minor feature changes here, I decided this would be best as a patch release 1.0.2.
Deployed on develop rc: https://dev.vegbank.org/rc?cite=doi:10.82902/J17P4J
What
Fixes #217 by ensuring that the citation text builder recognizes both dois that are prefixed with doi: and those without it.
Also updates all copy permalink buttons to specify https instead of a bare vegbank.org url and adds the ability to pass a specific non-vegbank citation url to them as well. The dataset detail view uses that ability to copy doi.org citations for datasets that have dois and identifiers.org citations for legacy accession codes. If neither pattern matches, it falls back to copying https://vegbank.org/cite/ds_code.
Finally, the detail panel css was updated to use dvh instead of just vh to hopefully ensure all details scroll into view (works on my phone now on the dev rc site!). When I opened your linked problem page on mobile the browser controls obscured part of the view and prevented me from actually seeing the error.
Why
Because the api's create_dataset() saves an accession_code with the doi: prefix and this was breaking the application as documented in 217.
How
Testing and Documentation
All 1586 tests pass and check() has no errors, warnings, or notes!